Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use minimum font-size of 12px #992

Closed

Conversation

samford
Copy link
Member

@samford samford commented Sep 9, 2023

PageSpeed Insights and Lighthouse complain about text that's smaller than 12px (for mobile). Our styling for pre code and #information .credits both use a font-size that's smaller than 12px (11px and ~11.5281px respectively).

This increases the smallest font-size values in style.scss to 12px (80% in this case).

PageSpeed Insights and Lighthouse complain about text that's smaller
than 12px (for mobile). Our styling for `pre code` and `#information
.credits` both use a `font-size` that's smaller than 12px (11px and
~11.5281px respectively).

This increases the smallest `font-size` values in `style.scss` to
12px (80% in this case).
@samford samford force-pushed the accessibility/use-legible-font-size branch from dce55ea to 1b4ba70 Compare September 9, 2023 01:30
@samford samford changed the title Increase lowest font-size to 12px Use minimum font-size of 12px Sep 9, 2023
@MikeMcQuaid
Copy link
Member

@samford Can I see a before/after screenshot for this on desktop/mobile (and generally for anything that changes appearance)? Note that I may suggest this be scoped just to mobile depending on how it looks.

@samford
Copy link
Member Author

samford commented Sep 11, 2023

Can I see a before/after screenshot for this on desktop/mobile (and generally for anything that changes appearance)?

Here are full screenshots of the homepage on mobile and desktop, before and after. I took the mobile screenshots using an iPhone SE simulator (basically the lowest common resolution for mobile at 375px x 667px) and the desktop screenshots are 1024px wide. Safari created 2x images for both, so the images are notably larger than they would appear on a device.

[I've used img elements below to restrict the width of the images but how they appear will depend on your browser width. The desktop images are wider than the maximum that GitHub's layout expands to, so those will appear smaller than 1024px.]

Mobile, before

Screenshot of homepage on mobile, before changes

Mobile, after

Screenshot of homepage on mobile, after changes

Desktop, before

Screenshot of homepage on desktop, before changes

Desktop, after

Screenshot of homepage on desktop, after changes

@MikeMcQuaid
Copy link
Member

@samford Ironically: I think the desktop situation is better but mobile is worse after these changes. The install link is not even vaguely readable and the wget example hides more than it did before. Does the wget formula have usable scrollbars? If so, I'm fine with that being made bigger. I think the install copy-paste could perhaps just be hidden entirely on mobile because it's not actually useful there.

@samford
Copy link
Member Author

samford commented Sep 11, 2023

Does the wget formula have usable scrollbars?

The code elements are scrollable, though mobile (and macOS by default) doesn't show scroll bars until you scroll within the element. At least for me, when I'm reading code on a mobile device, I scroll [horizontally] and/or rotate the device to increase the width. GitHub uses a 12px font-size for code, so I'm pretty used to this behavior.

In landscape orientation, the homepage after changes looks like this:

Homepage on mobile (landscape) after changes

I think the install copy-paste could perhaps just be hidden entirely on mobile because it's not actually useful there.

Just to be sure, are you talking about the "copy to clipboard" button (.copyable button)? If so, hiding it below 480px kind of makes sense to me. That said, I just tried to manually copy the text on my iPhone and it's a little bit of a pain but part of that is because the text selection can extend to the clipboard emoji (unless we add user-select: none to .copyable button).

One thing worth mentioning is that iOS Safari's default padding is applying to the button, which is why it's wider on mobile than desktop:

Homepage install snippet on mobile after changes

If we set the left/right padding to 8px (closer to desktop), it ends up looking a little more reasonable:

Homepage install snippet on mobile after changes, with 8px border on copy button

Without the copy button it looks like:

Homepage install snippet on mobile after changes, with no copy button

@MikeMcQuaid
Copy link
Member

Just to be sure, are you talking about the "copy to clipboard" button (.copyable button)? If so, hiding it below 480px kind of makes sense to me. That said, I just tried to manually copy the text on my iPhone and it's a little bit of a pain but part of that is because the text selection can extend to the clipboard emoji (unless we add user-select: none to .copyable button).

I'm saying we hide the entire URL and perhaps show something else instead. It's not useful one mobile.

@maxim-belkin
Copy link
Member

@samford, you can target specific screen sizes by placing your changes inside @media only screen and (max-width: 480px) (replace 480 with appropriate value, or max-width to min-width). There are a few places in the file that you're changing that do that.

@samford
Copy link
Member Author

samford commented Sep 13, 2023

I'm saying we hide the entire URL and perhaps show something else instead. It's not useful one mobile.

If we're generally fine with these font-size changes, how do you feel about handling any changes to the "Install Homebrew" section in a separate (follow-up) PR? I don't have any ideas on what we could display instead (i.e., the text after the shell snippet doesn't make much sense if we only hide the code). I'm happy to incorporate changes here if you have ideas, though.


you can target specific screen sizes by placing your changes inside @media only screen and (max-width: 480px) (replace 480 with appropriate value, or max-width to min-width). There are a few places in the file that you're changing that do that.

I'm aware (I do web development for a living 😆) but my thinking was that the font-size increases weren't undesirable changes for higher resolutions as well. #information .credits only increases by around half a pixel (and is arguably more legible as a result), so I think that's fine everywhere.

If we want to scope the pre code increase (from 11px to 12px) only to mobile, then we would probably need a higher breakpoint than 480px. That said, GitHub uses 12px for code and I don't see much of an issue with us using it everywhere, too. If the two-column layout hides too much of the code, we could always switch to a one-column layout at a higher width than the current 700px breakpoint.

@MikeMcQuaid
Copy link
Member

MikeMcQuaid commented Sep 13, 2023

If we're generally fine with these font-size changes, how do you feel about handling any changes to the "Install Homebrew" section in a separate (follow-up) PR?

I'm not, sorry. As-is this is a "make linter happy" change that makes things worse rather than better for users. As before, I'd happily see a redesign of this part of the page that means this isn't needed for install URL, something that's not going to be copy/pasted on an iPhone.

I don't have any ideas on what we could display instead (i.e., the text after the shell snippet doesn't make much sense if we only hide the code). I'm happy to incorporate changes here if you have ideas, though.

Would you like the idea of what it should look like or the code or both or neither?

That said, GitHub uses 12px for code and I don't see much of an issue with us using it everywhere, too.

We're not really displaying code in the same way as GitHub here, though.

@samford
Copy link
Member Author

samford commented Sep 13, 2023

Would you like the idea of what it should look like or the code or both or neither?

A concrete idea and any text changes would be good (i.e., I'm fine to take care of any implementation). At the moment, all of the text in the "Install Homebrew" section either references the install snippet or talks about the pkg installer.

That said, though mobile users wouldn't have any direct use for the install information on their device, I'm a little wary of assuming that it wouldn't be useful to them in general. They wouldn't be copy/pasting the snippet or using the pkg installer but I'm not totally convinced that the install information isn't useful to them at all.

For example, someone may load the homepage for the first time on mobile (e.g., checking it while they're away from their computer after hearing about the project) and the install instructions give them an idea of how brew is installed (so they know what to do when they're on their computer again). Maybe it's just that I don't know what we would replace the install text with on mobile at the moment, though.

@MikeMcQuaid
Copy link
Member

Would you like the idea of what it should look like or the code or both or neither?

A concrete idea and any text changes would be good (i.e., I'm fine to take care of any implementation). At the moment, all of the text in the "Install Homebrew" section either references the install snippet or talks about the pkg installer.

@samford Cool, I would replace all text in the "Install Homebrew" section (i.e. everything before "What Does Homebrew Do?") with a single line of text saying something like

Homebrew's [installation script](https://github.com/Homebrew/install/blob/HEAD/install.sh) is run from a macOS Terminal or Linux shell prompt. 
Check this page on your computer for the copy-pasteable command. 
Alternatively, read about other [Homebrew's other installation options](https://docs.brew.sh/Installation).

I think those two links are the two things people might want to look at and they are fairly accessible on mobile compared to trying to extract anything meaningful from the text string with curl in it.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale label Oct 6, 2023
@samford
Copy link
Member Author

samford commented Oct 6, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

I'll try to remember to revisit this over the weekend when I have a moment.

@github-actions github-actions bot removed the stale label Oct 6, 2023
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale label Oct 28, 2023
@github-actions github-actions bot closed this Nov 4, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants